✨ Add a shuffle argument ✨ #2169
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implementation in `source_file()`, and all the other work is just ensuring that argument gets threaded back up to user facing functions. Fixes #1942
|
@jennybc would you mind taking a look at this? Only because it's related to your pet peeve of not being able to run test files out of order. You might want to run it on testthat itself and tell me if you think I need to change testthat code. |
jennybc
approved these changes
Aug 10, 2025
There was a problem hiding this comment.
LGTM
My main assessment was by running the testthat tests shuffled, which was pretty interesting! I think this is an interesting way for folks to examine the health of their test suite. I predict that there will be more legitimate "wins" from this for your average package in the wild (vs in testthat).
I did this at the top-level of testthat source:
R --quiet --no-restore --no-save -e 'testthat::set_max_fails(Inf);devtools::test(shuffle = TRUE)'
Main findings
- Chaos in snapshot files that simply comes from relocation. I suppose this could obscure an actual issue relating to snapshots? Hard to say. But the few true "wins" I had here were revealed by snapshots, so there might be some more problems lurking that are just less obvious than the couple that I caught.
- Duplicated test names caused crosstalk between snapshots. The commit I pushed contains some fixes of this, so that is the actual win here in testthat.
- Externally defined objects are not found. testthat makes modest intentional use of this; I don't think I uncovered any unintentional dependency on code-outside-the-test.
- External setup. testhat makes some use of, e.g., file-wide local edition.
Below I'll show the output related to ☝️
Tests depend on code outside of test
External object
These all looked very intentional and sound to me.
Error (test-describe.R:36:5): describe: should be possible to use variables from outer environments
Error in `eval(code, test_env)`: object 'someExternalVariable' not found
Error (test-expect-output.R:15:3): expect = string checks for match
Error in `g()`: could not find function "g"
Error (test-expect-output.R:5:3): expect = NA checks for no output
Error in `f()`: could not find function "f"
Error (test-reporter-debug.R:111:3): browser() is called for the correct frame for warnings
Error in `get_vars_from_debug_reporter(1, fun_1)`: could not find function "get_vars_from_debug_reporter"
Error (test-srcrefs.R:55:3): line numbers captured for skip()s and stops()
Error in `srcref_line({ test_that("simple", { skip("Not this time") }) })`: could not find function "srcref_line"
Error (test-context.R:31:3): contexts are opened, then closed
Error in `eval(code, test_env)`: object 'CountReporter' not found
Error (test-try-again.R:9:3): tries multiple times
Error in `succeed_after(3)`: could not find function "succeed_after"
External setup
File-wide local edition:
Warning (test-expect-known.R:69:3): correctly matches to a file
`expect_known_value()` was deprecated in the 3rd edition.
Again, clearly intentional.
Duplicated test names cause snapshot crosstalk FIXED
This lead to some actual improvements in the tests.
Failure (test-expect-inheritance.R:86:3): expect_s3_class validates its inputs
Snapshot of code has changed: ...
Failure (test-expect-inheritance.R:52:3): expect_s3_class validates its inputs
Snapshot of code has changed: ...
Failure (test-skip.R:55:3): skip_if_not_installed() works as expected
Snapshot of `x` has changed:
Failure (test-skip.R:43:3): skip_if_not_installed() works as expected
Snapshot of `x` has changed:
Failure (test-skip.R:44:3): skip_if_not_installed() works as expected
Snapshot of `x` has changed:
Chaos in snapshot files
Just general shuffling, which makes sense.
Co-authored-by: Jennifer (Jenny) Bryan <[email protected]>
|
Thanks for the careful analysis and exploration! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implementation in
source_file(), and all the other work is just ensuring that argument gets threaded back up to user facing functions.Fixes #1942